Conversation
✅ Deploy Preview for fluffy-chebakia-3fa329 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| node under ``/images``, the boot firmware checks for a | ||
| :index:`image-reference subnode` with that name directly under the | ||
| configuration node. The subnode lets a configuration override the load | ||
| and/or entry address of the referenced image without duplicating the image |
There was a problem hiding this comment.
Can't the same be readily achieved with external data? Have multiple nodes, each with its own load/entry addresses and have them all point at the same external data region.
There was a problem hiding this comment.
Hi @a3f ,
Thanks for the suggestion! If I understand correctly, the external data mechanism (mkimage -E) does move binary payloads outside the FIT blob, but it doesn't help with the duplication issue here. If we have M image nodes referencing the same binary, fit_extract_data() in tools/fit_image.c iterates every /images subnode independently and copies each one's data property to a new sequential position in the external area. There is no content deduplication, so the same kernel binary ends up stored M times.
The reference subnode approach avoids this entirely, a single /images/kernel-1 node holds the binary once, and each configuration carries a small subnode (image, load, entry) expressing where to put it. No deduplication logic is needed.
There was a problem hiding this comment.
There is no content deduplication, so the same kernel binary ends up stored M times.
Sounds like something that can be changed in mkimage?
No deduplication logic is needed.
The way I see it:
- We can patch
mkimageand existent bootloaders will just be able to make use of it and no spec change is needed - We complicate the spec and have to patch bootloaders to make use of this new mechanism
Option 1. is clearly better in my eyes.
There was a problem hiding this comment.
Sounds like something that can be changed in mkimage?
Yes, I think so
We can patch mkimage and existent bootloaders will just be able to make use of it and no spec change is needed
If I understand correctly, multiple image nodes sharing the same data-offset is currently undefined behaviour in the FIT spec. The spec defines data-offset only as
data-offset
Offset of the data in a separate image store. The image store is placed
immediately after the last byte of the device tree binary, aligned to a
4-byte boundary. This is mandatory if external data is used, with an offset.
with no statement on whether offsets must be unique, whether sharing is permitted, or how tools should handle it.
To make this approach well-defined, I feel like we should put it into the spec, and would need to address at least:
- explicitly permitting multiple nodes to reference the same region via identical data-offset and data-size
- defining loader behaviour, meaning a loader MUST be able to read the same region more than once for different configurations
- defining hash verification, where each node carries its own hash sub-node covering the shared region and verification is performed independently per node
- defining signing-tool behaviour, requiring that a tool re-signing a FIT with shared regions MUST preserve the sharing across the import/export cycle.
So in my view, we may need to consider other pros and cons for a final decision unless there is another better approach
Currently what I have in my mind why my approach may be better are
- Long-term Maintainability
Reference subnode: Adding a new board means adding a new config with a small subnode (3-4 properties). The/imagessection stays stable regardless of how many boards you support. If you update the kernel binary, you change one image node and all boards pick it up. The ITS source directly expresses the intent: "for this board, use this binary, but place it here."External data with deduplication: Adding a new board means adding a full new image node in/images, repeatingtype,arch,os,compression,load,entry, and ahash-1subnode. For M boards and N image types you have M*N full image nodes. Updating the kernel binary means updating M nodes. The relationship between board variants of the same binary is implicit in naming conventions, not structural. The/imagessection becomes harder to read as board count grows.
- Extensibility
Reference subnode: The subnode is a general-purpose container for any per-config override. Future additions (per-config boot arguments, compatible string overrides, compression hints) can be added as new properties in the same subnode with no new mechanism needed or probably small changes.External data with deduplication: Only addresses the load/entry address problem by placing them in separate image nodes. Any other per-config override would require a completely different mechanism. It is a relatively narrow solution to one specific problem rather than a general model.
a3f
left a comment
There was a problem hiding this comment.
Please split off the unrelated Image-reference subnodes commit into its own PR
duh! sorry about that. I was reviewing your PR and then pushed the wrong branch |
| @@ -483,7 +483,10 @@ value | |||
|
|
|||
| hashed-nodes | |||
| A list of nodes which were :index:`hashed <pair: nodes; hashed>` by the | |||
There was a problem hiding this comment.
| A list of nodes which were :index:`hashed <pair: nodes; hashed>` by the | |
| An unsorted list of nodes which were :index:`hashed <pair: nodes; hashed>` by the |
We tripped over the fact yesterday that mkimage(1) sorts hashed-nodes according to sign-images order instead of actual hashing order (which is device tree order).
There was a problem hiding this comment.
I meant to add unsorted here (for hashed-nodes), but adding it for sign-images in addition is fine as well.
There was a problem hiding this comment.
OK, please check again
There was a problem hiding this comment.
fef1b5b to
fb220ac
Compare
The hashed-nodes property is not itself protected by a hash, so a loader cannot rely on it for validation. Clarify that it serves only as a hint for the signer. Also mention that both hash-nodes and sign-images are not necessarily in sorted order. Signed-off-by: Simon Glass <sjg@chromium.org>
a3f
left a comment
There was a problem hiding this comment.
LGTM. It's unfortunate that the spec doesn't document how the hash is derived, but that doesn't invalidate this PR.
The hashed-nodes property is not itself protected by a hash, so
a loader cannot rely on it for validation. Clarify that it serves
only as a hint for the signer.